Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR migrates ARC’s runtime baseline to Python 3.14 and modernizes the codebase accordingly (dependency pins, version gating, and widespread typing updates).
Changes:
- Bump Python target/runtime metadata (3.12 → 3.14) and update dependency minimums (e.g., Cython/RDKit) across env/packaging artifacts.
- Update Python-version gate logic and remove deprecated/legacy Python constructs.
- Replace deprecated
typinggenerics with built-in generics /X | Noneannotations across many modules.
Reviewed changes
Copilot reviewed 76 out of 77 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| utilities.py | Updates runtime Python version gate messaging/threshold. |
| requirements.txt | Raises minimum Python/RDKit/Cython versions (but see review comments). |
| pyproject.toml | Adds Python version metadata for packaging. |
| environment.yml | Pins conda env to Python 3.14 and updates Cython/RDKit pins. |
| Dockerfile | Builds ARC micromamba env with Python 3.14. |
| arc/utils/scale.py | Modernizes typing annotations and removes legacy typing imports. |
| arc/statmech/factory.py | Modernizes typing annotations for factory registration and adapter creation. |
| arc/species/vectors.py | Modernizes typing annotations for vector utilities. |
| arc/scripts/rmg_thermo.py | Updates typing annotations in the RMG thermo helper script. |
| arc/scripts/rmg_kinetics.py | Updates typing annotations in the RMG kinetics helper script. |
| arc/scripts/pipe_worker.py | Updates typing annotations in the pipe-mode worker script. |
| arc/scripts/common.py | Updates typing annotations for YAML I/O helpers. |
| arc/processor.py | Updates typing annotations around postprocessing helpers. |
| arc/parser/factory.py | Modernizes adapter registration typing. |
| arc/parser/adapters/yaml.py | Modernizes typing annotations for YAML parsing adapter. |
| arc/parser/adapters/xtb.py | Modernizes typing annotations for xTB parsing adapter. |
| arc/parser/adapters/terachem.py | Modernizes typing annotations for TeraChem parsing adapter. |
| arc/parser/adapters/qchem.py | Modernizes typing annotations for Q-Chem parsing adapter. |
| arc/parser/adapters/psi_4.py | Modernizes typing annotations for Psi4 parsing adapter. |
| arc/parser/adapters/orca.py | Modernizes typing annotations for ORCA parsing adapter. |
| arc/parser/adapters/molpro.py | Modernizes typing annotations for Molpro parsing adapter. |
| arc/parser/adapters/cfour.py | Modernizes typing annotations for CFOUR parsing adapter. |
| arc/parser/adapter.py | Modernizes abstract adapter typing for ESS parsers. |
| arc/output.py | Modernizes typing annotations while writing consolidated output.yml. |
| arc/molecule/resonance.py | Updates typing annotations for resonance utilities. |
| arc/molecule/molecule_test.py | Replaces deprecated assertDictEqual usage. |
| arc/molecule/draw.py | Updates typing annotations for drawing utilities. |
| arc/mapping/driver.py | Updates typing annotations for reaction mapping utilities. |
| arc/main.py | Modernizes typing annotations in ARC main class and helpers. |
| arc/level.py | Updates typing annotations and switches to collections.abc.Iterable. |
| arc/job/ssh.py | Modernizes typing annotations for SSH client utilities. |
| arc/job/pipe/pipe_state.py | Modernizes typing annotations for pipe state/records. |
| arc/job/pipe/pipe_planner.py | Modernizes typing annotations for pipe task planning. |
| arc/job/pipe/pipe_coordinator.py | Modernizes typing annotations for coordinating pipe runs. |
| arc/job/local.py | Modernizes typing annotations for local execution helpers. |
| arc/job/factory.py | Modernizes typing annotations for job adapter factory/registration. |
| arc/job/adapters/xtb_adapter.py | Modernizes typing annotations for xTB job adapter. |
| arc/job/adapters/ts/xtb_gsm.py | Modernizes typing annotations for xTB-GSM TS adapter. |
| arc/job/adapters/ts/orca_neb.py | Modernizes typing annotations for ORCA NEB TS adapter. |
| arc/job/adapters/ts/kinbot_ts.py | Modernizes typing annotations for KinBot TS adapter. |
| arc/job/adapters/ts/gcn_ts.py | Modernizes typing annotations for GCN TS adapter. |
| arc/job/adapters/ts/autotst_ts.py | Modernizes typing annotations for AutoTST TS adapter. |
| arc/job/adapters/torch_ani.py | Modernizes typing annotations for TorchANI adapter/scripts. |
| arc/job/adapters/terachem.py | Modernizes typing annotations for TeraChem job adapter. |
| arc/job/adapters/scripts/xtb_gsm/tm2orca.py | Removes dead __future__ import usage. |
| arc/job/adapters/scripts/tani_script.py | Modernizes typing annotations in helper script. |
| arc/job/adapters/scripts/autotst_script.py | Modernizes typing annotations in helper script. |
| arc/job/adapters/qchem.py | Modernizes typing annotations for Q-Chem job adapter. |
| arc/job/adapters/psi_4.py | Modernizes typing annotations for Psi4 job adapter. |
| arc/job/adapters/orca.py | Modernizes typing annotations for ORCA job adapter. |
| arc/job/adapters/obabel.py | Modernizes typing annotations for OpenBabel job adapter. |
| arc/job/adapters/molpro.py | Modernizes typing annotations for Molpro job adapter. |
| arc/job/adapters/mockter.py | Modernizes typing annotations for mock adapter. |
| arc/job/adapters/gaussian.py | Modernizes typing annotations for Gaussian job adapter. |
| arc/job/adapters/common.py | Modernizes typing annotations for shared job-adapter helpers. |
| arc/job/adapters/cfour.py | Modernizes typing annotations for CFOUR job adapter. |
| arc/job/adapter.py | Modernizes typing annotations for the base JobAdapter API. |
| arc/checks/common.py | Modernizes typing annotations for common checks helpers. |
Comments suppressed due to low confidence (1)
arc/output.py:37
write_output_yml()still annotates parameters asDict/List, but those names are no longer imported (and there is no postponed annotation evaluation). This will raiseNameErrorwhen importingarc.output. Replace these with built-in generics (dict[...],list[...]) or re-import the typing aliases, and apply the same fix to otherDict/Listannotations in this file.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _output_command_error_message(command: list[str], | ||
| error: subprocess.CalledProcessError, | ||
| logging_func: Union[logger.warning, logger.error], | ||
| logging_func: logger.warning | logger.error, | ||
| ) -> None: |
There was a problem hiding this comment.
_output_command_error_message() uses logging_func: logger.warning | logger.error as a type annotation. Since this module does not use postponed annotation evaluation, this expression is evaluated at import time and will raise TypeError (bitwise-or between bound methods). Use a Callable[[object], object]/Callable[..., Any] annotation instead (or stringize via from __future__ import annotations).
|
|
||
|
|
||
| def combine_parameters(input_dict: dict, terms: list) -> Tuple[dict, List]: | ||
| def combine_parameters(input_dict: dict, terms: list) -> tuple[dict, List]: |
There was a problem hiding this comment.
combine_parameters() annotates its return type as tuple[dict, List], but List is no longer imported in this module. Because annotations are evaluated at function definition time here, importing this module will raise NameError: name 'List' is not defined. Replace List with a built-in list[...] type (or re-import List).
| def combine_parameters(input_dict: dict, terms: list) -> tuple[dict, List]: | |
| def combine_parameters(input_dict: dict, terms: list) -> tuple[dict, list]: |
| @abstractmethod | ||
| def parse_frequencies(self) -> Optional['np.ndarray']: | ||
| def parse_frequencies(self) -> 'np.ndarray' | None: | ||
| """ |
There was a problem hiding this comment.
This module now uses return annotations like 'np.ndarray' | None (e.g. parse_frequencies). Without postponed annotation evaluation (from __future__ import annotations), this is evaluated at import time and raises TypeError because it's a str | None expression. Use np.ndarray | None (and import numpy at runtime) or use a single quoted forward reference like 'np.ndarray | None' / enable postponed evaluation.
|
I can't seem to properly comment on the line, and I know this has gone back to draft and I was not requested to review but I did see that the output.py still has Dict in it without either being imported or changed over |
alongd
force-pushed
the
py14
branch
10 times, most recently
from
769e011 to
291e2f9
Compare
|
Openbable seems to be the only package that inhibits the Py14 upgrade. The OB repo hasn't been updated since Dec 2024. There is a PR (number 2809) from July 2025 to bump into Py13, the discussion hasn't advanced since then. |
alongd
force-pushed
the
py14
branch
2 times, most recently
from
ef8e15e to
c894c82
Compare
| _result = _sp.run([OB_PYTHON, '-c', 'from openbabel import openbabel; ' | ||
| 'assert openbabel.OBForceField.FindForceField("MMFF94s") is not None'], |
There was a problem hiding this comment.
Could put a + like github security says, or make a var called command first
cmd = (
'from openbabel import openbabel; '
'assert openbabel.OBForceField.FindForceField("MMFF94s") is not None'
)
And then place it in the run function
alongd
force-pushed
the
py14
branch
3 times, most recently
from
bebda2b to
9ad661c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #867 +/- ##
==========================================
- Coverage 60.48% 52.27% -8.21%
==========================================
Files 102 102
Lines 31102 31114 +12
Branches 8104 8110 +6
==========================================
- Hits 18813 16266 -2547
- Misses 9952 12834 +2882
+ Partials 2337 2014 -323
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
alongd
force-pushed
the
py14
branch
3 times, most recently
from
5b38b59 to
7651a07
Compare
Update version pins across environment.yml, requirements.txt, Dockerfile, and pyproject.toml. Source OpenBabel from the danagroup channel (conda-forge lacks a Python 3.14 build).
Replace deprecated typing generics with built-in equivalents across 70+ files: - List/Dict/Tuple/Set/Type → list/dict/tuple/set/type - Optional[X] → X | None, Union[X, Y] → X | Y - Sequence/Iterable/Callable → from collections.abc - typing.Match → re.Match - np.array → np.ndarray in type annotations Scripts that run as subprocesses in older Python envs use from __future__ import annotations or typing equivalents.
- generate-run-shell: true so micromamba-shell activates arc_env - Explicit condarc channels (conda-forge + danagroup) - ase fallback install step for solver edge cases - Import verification step before docs build
- Conformers: updated RDKit force field expected values/coordinates - xTB-GSM: updated HNO/HON optimized coordinates - Reaction: updated dict key ordering and species coordinates - OB SMILES C(1)CC(1) → C1CC1 in species test - Various: path handling, deprecated assertDictEqual removal
Add Python 3.14 badge to README. Update version references in docs from Python 3.7 to 3.14. conformers test: The issue: from_adjacency_list calls update() which may call sort_atoms(). Different Cython compilation could produce different sort tiebreaking. The fix: make the test find atoms by structure, not indices
The danagroup OpenBabel conda build doesn't ship activate scripts that set BABEL_LIBDIR/BABEL_DATADIR. Auto-detect these paths from the conda prefix in settings.py (where other env config lives). The OB adapter subprocess inherits the env vars automatically, removing the need for manual detection in execute_incore().
- common.py get_git_branch: add explicit return when no branch found - common.py is_same_pivot: add explicit return False for non-matching - torch_ani.py write_input_file: match parent class signature
The arc_env creation inside Docker couldn't solve for Python 3.14 because the danagroup channel (needed for OpenBabel) wasn't passed to micromamba. Also removed the redundant python=3.14 pin since it's already in environment.yml. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| self.assertIsNone(sched.species_dict[ts_label].ts_checks['NMD']) | ||
| self.assertIsNone(sched.species_dict[ts_label].ts_checks['E0']) | ||
|
|
||
| # Verify rotors convergence flag preserved as True (not blanket-reset to False). |
There was a problem hiding this comment.
Was this test completely removed or moved to another script?
| self.assertTrue(sched.output[ts_label]['job_types']['rotors']) | ||
|
|
||
| @patch('arc.scheduler.Scheduler.run_opt_job') | ||
| def test_switch_ts_rotors_reset(self, mock_run_opt): |
There was a problem hiding this comment.
Same question (removed or moved?)
Bump the target runtime from 3.12 to 3.14 and modernize the codebase accordingly.
RMG's separate Python 3.9 environment for suprocess scripts (e.g. Arkane) is unchanged.